feat: add magic router-compatible RPC methods to validator #704
Conversation
📝 WalkthroughWalkthroughAdds support for three Magic Router JSON-RPC methods: Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
magicblock-aperture/src/requests/http/get_blockhash_for_accounts.rs(1 hunks)magicblock-aperture/src/requests/http/get_delegation_status.rs(1 hunks)magicblock-aperture/src/requests/http/get_routes.rs(1 hunks)magicblock-aperture/src/requests/http/mod.rs(1 hunks)magicblock-aperture/src/requests/mod.rs(2 hunks)magicblock-aperture/src/server/http/dispatch.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/http/get_program_accounts.rs:17-25
Timestamp: 2025-10-21T13:06:38.900Z
Learning: The magicblock validator does not support ledger forking, so commitment-based state queries (processed/confirmed/finalized) are not applicable. RPC methods can safely ignore commitment and minContextSlot parameters from Solana RPC config objects.
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-aperture/src/requests/http/mod.rsmagicblock-aperture/src/requests/mod.rs
🧬 Code graph analysis (1)
magicblock-aperture/src/requests/http/get_delegation_status.rs (1)
magicblock-aperture/src/requests/mod.rs (1)
params(27-31)
🔇 Additional comments (8)
magicblock-aperture/src/server/http/dispatch.rs (1)
229-231: LGTM!The routing for the three new Magic Router compatibility methods follows the established pattern. The sync/async distinction is appropriate:
GetRoutesandGetBlockhashForAccountsare synchronous since they don't perform I/O, whileGetDelegationStatusis correctly awaited as it callsread_account_with_ensure.magicblock-aperture/src/requests/http/mod.rs (1)
305-309: LGTM!The module declarations are well-organized with a clear comment indicating their purpose. Grouping the Magic Router compatibility methods separately improves discoverability.
magicblock-aperture/src/requests/http/get_blockhash_for_accounts.rs (1)
1-17: LGTM!The implementation correctly aliases
getLatestBlockhashfor API compatibility. The docstring clearly explains that this is an intentional simplification for the validator context (single blockhash source). Based on learnings, the magicblock validator doesn't support ledger forking, so returning the latest blockhash is the appropriate behavior.magicblock-aperture/src/requests/http/get_routes.rs (1)
1-15: LGTM!The implementation correctly returns an empty routes list for API compatibility, as a validator is not a router. The docstring clearly documents this intentional behavior.
magicblock-aperture/src/requests/http/get_delegation_status.rs (1)
28-38: LGTM!The account retrieval and delegation status derivation logic is correct. Using
unwrap_or(false)for non-existent accounts is appropriate—an account that doesn't exist cannot be delegated. The response payload format matches the documented API contract.magicblock-aperture/src/requests/mod.rs (3)
80-85: LGTM! Well-documented enum additions.The three new HTTP method variants are clearly documented with accurate descriptions of their validator-side behavior. The comments correctly indicate that:
GetRoutesis mocked (returns empty array for compatibility)GetBlockhashForAccountsaliases the existinggetLatestBlockhashGetDelegationStatusexposes a simple delegation flag
149-151: LGTM! String mappings are correct.The string mappings correctly use camelCase convention and match the method names expected by the Magic Router SDK.
91-102: AI summary is inaccurate: WebSocket variants were not added despite claims.The AI summary states that
GetRoutes,GetBlockhashForAccounts, andGetDelegationStatuswere added toJsonRpcWsMethod, but the diff shows no changes to this enum. Only HTTP variants were added.If WebSocket support is required for these methods (for Magic Router or other clients), the corresponding
JsonRpcWsMethodvariants and handlers must be added.
magicblock-aperture/src/requests/http/get_blockhash_for_accounts.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
magicblock-aperture/src/requests/http/get_delegation_status.rs(1 hunks)magicblock-aperture/src/requests/http/mocked.rs(1 hunks)magicblock-aperture/src/requests/http/mod.rs(1 hunks)magicblock-aperture/src/server/http/dispatch.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/http/get_program_accounts.rs:17-25
Timestamp: 2025-10-21T13:06:38.900Z
Learning: The magicblock validator does not support ledger forking, so commitment-based state queries (processed/confirmed/finalized) are not applicable. RPC methods can safely ignore commitment and minContextSlot parameters from Solana RPC config objects.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
📚 Learning: 2025-12-03T09:36:01.527Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/src/remote_account_provider/mod.rs:1350-1353
Timestamp: 2025-12-03T09:36:01.527Z
Learning: Repo: magicblock-labs/magicblock-validator
File: magicblock-chainlink/src/remote_account_provider/mod.rs
Context: consolidate_fetched_remote_accounts
Learning: For unexpected result counts (>2), the project prefers logging an error and returning an empty Vec over panicking; acceptable during development per maintainer (Dodecahedr0x).
Applied to files:
magicblock-aperture/src/requests/http/mod.rsmagicblock-aperture/src/requests/http/get_delegation_status.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rs
📚 Learning: 2025-11-07T14:20:31.457Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: magicblock-chainlink/src/remote_account_provider/chain_pubsub_actor.rs:457-495
Timestamp: 2025-11-07T14:20:31.457Z
Learning: In magicblock-chainlink/src/remote_account_provider/chain_pubsub_client.rs, the unsubscribe closure returned by PubSubConnection::account_subscribe(...) resolves to () (unit), not a Result. Downstream code should not attempt to inspect an unsubscribe result and can optionally wrap it in a timeout to guard against hangs.
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rs
📚 Learning: 2025-10-26T16:53:29.820Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: magicblock-chainlink/src/remote_account_provider/mod.rs:134-0
Timestamp: 2025-10-26T16:53:29.820Z
Learning: In magicblock-chainlink/src/remote_account_provider/mod.rs, the `Endpoint::separate_pubsub_url_and_api_key()` method uses `split_once("?api-key=")` because the api-key parameter is always the only query parameter right after `?`. No additional query parameter parsing is needed for this use case.
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rs
🧬 Code graph analysis (3)
magicblock-aperture/src/requests/http/mod.rs (1)
magicblock-aperture/src/requests/http/get_delegation_status.rs (1)
get_delegation_status(12-30)
magicblock-aperture/src/requests/http/mocked.rs (1)
magicblock-aperture/src/requests/payload.rs (2)
encode_no_context(85-100)encode_no_context(138-148)
magicblock-aperture/src/requests/http/get_delegation_status.rs (1)
magicblock-aperture/src/requests/mod.rs (1)
params(27-31)
🔇 Additional comments (3)
magicblock-aperture/src/requests/http/mod.rs (1)
305-307: Module wiring forget_delegation_statuslooks consistentThe new module export and Router-compatibility comment align with existing handler organization and keep router-specific logic clearly separated.
magicblock-aperture/src/requests/http/get_delegation_status.rs (1)
1-31:get_delegation_statushandler is concise and reuses existing helpers appropriatelyThe handler cleanly uses
parse_params!/some_or_err!,read_account_with_ensure, andAccountSharedData::delegated()to derive a minimal{ "isDelegated": bool }response, defaulting safely tofalsewhen the account is absent or cannot be ensured. This matches the “validator-only, no router-style resolution” design.Please just confirm that the Magic Router SDK / client code treats any extra fields as optional and is happy with this minimal object shape.
magicblock-aperture/src/server/http/dispatch.rs (1)
229-233: VerifygetBlockhashForAccountsparam compatibility withget_latest_blockhashThe routing directly delegates
GetBlockhashForAccountstoget_latest_blockhash(request)without visible param transformation. Confirm thatget_latest_blockhashaccepts the param shape sent by Magic Router SDK'sgetBlockhashForAccountscalls (which likely differ from standardgetLatestBlockhashparams). If param shapes are incompatible, add normalization before delegation to achieve the intended drop-in compatibility.
|
@Shubbu03 thanks for addressing the comments. Can you please resolve the issues with the CI and we can merge this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
magicblock-aperture/src/requests/http/get_delegation_status.rs(1 hunks)magicblock-aperture/src/requests/http/mocked.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/http/get_program_accounts.rs:17-25
Timestamp: 2025-10-21T13:06:38.900Z
Learning: The magicblock validator does not support ledger forking, so commitment-based state queries (processed/confirmed/finalized) are not applicable. RPC methods can safely ignore commitment and minContextSlot parameters from Solana RPC config objects.
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rsmagicblock-aperture/src/requests/http/mocked.rs
📚 Learning: 2025-11-07T14:20:31.457Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 621
File: magicblock-chainlink/src/remote_account_provider/chain_pubsub_actor.rs:457-495
Timestamp: 2025-11-07T14:20:31.457Z
Learning: In magicblock-chainlink/src/remote_account_provider/chain_pubsub_client.rs, the unsubscribe closure returned by PubSubConnection::account_subscribe(...) resolves to () (unit), not a Result. Downstream code should not attempt to inspect an unsubscribe result and can optionally wrap it in a timeout to guard against hangs.
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rs
📚 Learning: 2025-12-03T09:36:01.527Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/src/remote_account_provider/mod.rs:1350-1353
Timestamp: 2025-12-03T09:36:01.527Z
Learning: Repo: magicblock-labs/magicblock-validator
File: magicblock-chainlink/src/remote_account_provider/mod.rs
Context: consolidate_fetched_remote_accounts
Learning: For unexpected result counts (>2), the project prefers logging an error and returning an empty Vec over panicking; acceptable during development per maintainer (Dodecahedr0x).
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rsmagicblock-aperture/src/requests/http/mocked.rs
📚 Learning: 2025-10-26T16:53:29.820Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: magicblock-chainlink/src/remote_account_provider/mod.rs:134-0
Timestamp: 2025-10-26T16:53:29.820Z
Learning: In magicblock-chainlink/src/remote_account_provider/mod.rs, the `Endpoint::separate_pubsub_url_and_api_key()` method uses `split_once("?api-key=")` because the api-key parameter is always the only query parameter right after `?`. No additional query parameter parsing is needed for this use case.
Applied to files:
magicblock-aperture/src/requests/http/get_delegation_status.rs
🧬 Code graph analysis (1)
magicblock-aperture/src/requests/http/get_delegation_status.rs (1)
magicblock-aperture/src/requests/mod.rs (1)
params(27-31)
🔇 Additional comments (5)
magicblock-aperture/src/requests/http/get_delegation_status.rs (5)
1-2: LGTM!Standard prelude import pattern consistent with other handlers in the codebase.
3-15: LGTM! Clear documentation of simplified validator behavior.The documentation appropriately clarifies that this is a simplified implementation that returns only the
isDelegatedboolean flag without resolving delegation records. This aligns with the PR objectives for validator-specific behavior that differs from the full Magic Router implementation.
16-19: LGTM! Proper use of helper macros.The parameter parsing correctly uses
parse_params!andsome_or_err!macros, following the pattern established inget_account_infoas requested in the previous review feedback.
28-30: LGTM!Clean response construction using the
json!macro and appropriate encoding method.
21-26: No changes needed. The code correctly implements the intended behavior.The
read_account_with_ensuremethod's side effect of fetching accounts from the reference cluster is explicitly intentional, as documented in the function's comment (lines 21-22) and the method documentation inget_account_info.rs. This pattern is consistently used across multiple RPC endpoints (get_balance,get_account_info,get_token_account_balance) to ensure clients receive current state by populating the localAccountsDbfrom the reference cluster when needed. The error handling strategy properly logs failures and continues with available data, matching project patterns.Likely an incorrect or invalid review comment.
|
@bmuddha i've pushed code to fix it, after running locally, but the 1st action - |
2 participants
Summary
This PR adds the Magic Router–specific RPC methods to the ephemeral validator so that the existing
Connection/ConnectionMagicRouterclient code can be reused against a validator endpoint.Changes
Extend JsonRpcHttpMethod and HttpDispatcher to support:
getRoutesgetBlockhashForAccountsgetDelegationStatusImplement validator-side handlers:
Fixes #658
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.